Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Initial implementation of v0 mangling #2615

Merged
merged 1 commit into from
Oct 16, 2023
Merged

Initial implementation of v0 mangling #2615

merged 1 commit into from
Oct 16, 2023

Conversation

tamaroning
Copy link
Contributor

@tamaroning tamaroning commented Sep 10, 2023 •
edited
Loading

Addresses #2635
Depends on #2633

Implemented mangling of basic paths.
Traits and generic ADTs cannot be mangled yet
I will add tests later.

Example

#[lang = "sized"]
pub trait Sized {}

impl<T: Sized> T {}

struct S<T> {
    a: T,
}

impl S<i32> {
    fn a() {}
}

fn generic_func<T>(_: T) {}

fn main() {
    generic_func::<S<usize>>(S { a: 1 });
}
...
crab1: note: Start mangling: <d::S::<i32>>::a
crab1: note: => _RNvMC5crateNtC1d1S1a
crab1: note: Start mangling: <d::S::<i32>>::a
crab1: note: => _RNvMC5crateNtC1d1S1a
crab1: note: Start mangling: d::main
crab1: note: => _RNvC1d4main
crab1: note: Added builtin lang_item: sized for S<usize>
crab1: note: Start mangling: d::generic_func
crab1: note: => _RINvC1d12generic_funcNtC1d1SE
crab1: note: Start mangling: d::generic_func
crab1: note: => _RINvC1d12generic_funcNtC1d1SE

changelog

gcc/rust/ChangeLog:

	* backend/rust-compile-context.h: Modify declaration.
	* backend/rust-mangle.cc (struct V0Path): New struct.
	(v0_path): New function.
	(legacy_mangle_name): Take Context as argument.
	(v0_numeric_prefix): Fix type strings.
	(v0_complex_type_prefix): New function.
	(v0_add_integer_62): Deleted
	(v0_integer_62): New function.
	(v0_add_opt_integer_62): Deleted.
	(v0_opt_integer_62): New function.
	(v0_add_disambiguator): Deleted.
	(v0_disambiguator): New function.
	(v0_type_prefix): Support more types.
	(v0_generic_args): New function.
	(v0_add_identifier): Deleted.
	(v0_identifier): New function.
	(v0_type_path): New function.
	(v0_function_path): New function.
	(v0_scope_path): New function.
	(v0_crate_path): New function.
	(v0_inherent_or_trait_impl_path): New function.
	(v0_mangle_item): Use v0_path.
	(Mangler::mangle_item): Take Context as argument.
	* backend/rust-mangle.h (class Context): Add forward declaration.
	* hir/tree/rust-hir-item.h: Fix include.

@tamaroning tamaroning force-pushed the impl-v0 branch 2 times, most recently from 389952a to b5c7f2b Compare September 20, 2023 04:42
@tamaroning tamaroning changed the title [wip] v0 mangling v0 mangling Sep 20, 2023
@tamaroning
Copy link
Contributor Author

tamaroning commented Sep 20, 2023 •
edited
Loading

deleted

CohenArthur
CohenArthur reviewed Sep 21, 2023
View reviewed changes
Copy link
Member

@CohenArthur CohenArthur left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like a great starting point to me - I assume you've compared the output strings with rustc?

gcc/rust/backend/rust-mangle.cc Outdated
rust_unreachable ();
std::string mangled;

path.iterate_segs ([&] (const Resolver::CanonicalPath &seg) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this lambda is very long and would benefit from being split up in multiple tinier functions - but this can be addressed later before the PR gets merged

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. Fixed in this PR.

gcc/rust/backend/rust-mangle.cc Outdated
Comment on lines 357 to 507
case HIR::ImplItem::FUNCTION: {
prefix = "N";
ns = "v";
HIR::Function *fn = static_cast<HIR::Function *> (impl_item);
if (!fn->get_generic_params ().empty ())
{
generic_prefix = "I";
generic_postfix = v0_generic_arg (ty) + "E";
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for example here you could factor this into a function, which would take an HIR::Function& as argument and do the required handling

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed.

@tamaroning
Copy link
Contributor Author

tamaroning commented Sep 21, 2023 •
edited
Loading

I assume you've compared the output strings with rustc?

Partially yes, but mangled symbols contain metadata so I checked by demangling mangled symbols with rustc_demangle.

Here is a wrapper of rustc_demangle I am I using this
https://online-demangler-iv5u2bxxb-tamaroning.vercel.app/

@tamaroning tamaroning force-pushed the impl-v0 branch 2 times, most recently from 401362e to c53ebc2 Compare September 21, 2023 14:54
@tamaroning tamaroning changed the title v0 mangling Initial implementation of v0 mangling Sep 21, 2023
@tamaroning tamaroning force-pushed the impl-v0 branch 2 times, most recently from 280285b to b5b600c Compare September 21, 2023 15:18
@tamaroning tamaroning marked this pull request as ready for review September 21, 2023 15:20
tamaroning
tamaroning commented Sep 21, 2023
View reviewed changes
gcc/rust/hir/tree/rust-hir-item.h
Comment on lines 24 to +25
#include "rust-common.h"
#include "rust-hir-expr.h"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lacking include

@tamaroning tamaroning mentioned this pull request Sep 22, 2023
Merged
@XVilka
Copy link

XVilka commented Sep 27, 2023

This rz-libdemangle library might be helpful for you too, we support Rust demangling implemented in pure C.

@CohenArthur CohenArthur requested a review from philberty October 2, 2023 15:13
@tamaroning
Copy link
Contributor Author

@CohenArthur
Hi! This pr has been rebased on master and is now ready for merge.
Your review seems to be done on #2637 ?

@tamaroning @CohenArthur
gccrs: Initial implementation of v0 mangling
64453cb 
gcc/rust/ChangeLog:

	* backend/rust-compile-context.h: Modify declaration.
	* backend/rust-mangle.cc (struct V0Path): New struct.
	(v0_path): New function.
	(legacy_mangle_name): Take Context as argument.
	(v0_numeric_prefix): Fix type strings.
	(v0_complex_type_prefix): New function.
	(v0_add_integer_62): Deleted
	(v0_integer_62): New function.
	(v0_add_opt_integer_62): Deleted.
	(v0_opt_integer_62): New function.
	(v0_add_disambiguator): Deleted.
	(v0_disambiguator): New function.
	(v0_type_prefix): Support more types.
	(v0_generic_args): New function.
	(v0_add_identifier): Deleted.
	(v0_identifier): New function.
	(v0_type_path): New function.
	(v0_function_path): New function.
	(v0_scope_path): New function.
	(v0_crate_path): New function.
	(v0_inherent_or_trait_impl_path): New function.
	(v0_mangle_item): Use v0_path.
	(Mangler::mangle_item): Take Context as argument.
	* backend/rust-mangle.h (class Context): Add forward declaration.
	* hir/tree/rust-hir-item.h: Fix include.

Signed-off-by: Raiki Tamura <[email protected]>
@CohenArthur
Copy link
Member

@tamaroning thanks for the ping :) merging now

@CohenArthur CohenArthur enabled auto-merge October 16, 2023 13:17
@CohenArthur CohenArthur added this pull request to the merge queue Oct 16, 2023
Merged via the queue into Rust-GCC:master with commit b6a420f Oct 16, 2023
9 checks passed
@tamaroning tamaroning mentioned this pull request Nov 4, 2023
Open
16 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@CohenArthur CohenArthur CohenArthur left review comments

@philberty philberty Awaiting requested review from philberty

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@tamaroning @XVilka @CohenArthur